Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Migrate wasmer-cli to clap from structopt #2890

Closed
wants to merge 1 commit into from

Conversation

silwol
Copy link
Contributor

@silwol silwol commented May 17, 2022

Description

Review

  • Add a short description of the change to the CHANGELOG.md file

rust-toolchain Outdated
@@ -1 +0,0 @@
1.59
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Undo this plz

Copy link
Member

@syrusakbary syrusakbary left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks good other than the removal of the rust-toolchain file.
Once it's fixed we should be good to merge

@silwol
Copy link
Contributor Author

silwol commented May 20, 2022

There's still a few TODOs in the change that I'd like to have clarified first, that's why it is still a draft MR. The removal of the toolchain file will not be part of the final version of this MR of course, as has nothing to do with it.

@epilys
Copy link
Contributor

epilys commented May 20, 2022

I'd like to see a binary size of clap and structopt as well, out of curiosity.

@epilys epilys added the priority-low Low priority issue label May 20, 2022
@ptitSeb
Copy link
Contributor

ptitSeb commented May 20, 2022

What about cli-compiler?

@silwol
Copy link
Contributor Author

silwol commented May 20, 2022

@ptitSeb you're right, had a newer version of the branch including cli-compiler on my disk, force-pushed it now.
@epilys will collect a few numbers regarding binary size.
Didn't expect the MR to get that much attention while still in draft state. :-)

@silwol
Copy link
Contributor Author

silwol commented Jun 10, 2022

@epilys here are the numbers for changes in binary size (measured on my x64 desktop computer with Debian 11, without llvm compiler enabled, but I think that shouldn't change a lot in this context).

For transparency, this is how I collected the information:

# make sure we start clean
rm -rf target

# build wasmer cli (cranelift + singlepass, no llvm)
cargo build --release --manifest-path lib/cli/Cargo.toml --features cranelift,singlepass --bin wasmer

# build wasmer-headless cli
cargo build --release --manifest-path lib/cli/Cargo.toml --features headless --bin wasmer-headless

# build cli-compiler
cargo build --release --manifest-path lib/cli-compiler/Cargo.toml

# show filesize before stripping
echo "before stripping"
ls -l target/release/wasmer{,-headless,-compiler}

# strip the binaries
strip target/release/wasmer{,-headless,-compiler}

# show filesize after stripping
echo "after stripping"
ls -l target/release/wasmer{,-headless,-compiler}

Raw release binaries

All raw numbers in the tabe are sizes in bytes.

structopt clap absolute change relative change
wasmer 18334632 18299344 -35288 -0.19%
wasmer-compiler 4896048 4839632 -56416 -1.15%
wasmer-headless 12309944 12268000 -41944 -0.34%

Stripped release binaries

All raw numbers in the tabe are sizes in bytes.

structopt clap absolute change relative change
wasmer 11744824 11707960 -36864 -0.31%
wasmer-compiler 1304984 1251736 -53248 -4.08%
wasmer-headless 6780424 6739464 -40960 -0.60%

@epilys epilys closed this Jul 21, 2022
@syrusakbary syrusakbary reopened this Jul 22, 2022
@silwol
Copy link
Contributor Author

silwol commented Aug 5, 2022

Superseded by #3079

@silwol silwol closed this Aug 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority-low Low priority issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants